Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use most recent format as default format #2319

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

bingingem
Copy link
Contributor

Followed DaWoblefet's suggested approach to just use the most recently selected format as the default format.
https://www.smogon.com/forums/threads/select-default-format.3670473/

@bingingem
Copy link
Contributor Author

bingingem commented Jan 19, 2025

There's currently an issue where the first team in the teambuilder is selected on non-random formats. It's because a non-undefined teamIndex is being passed into renderTeams (""), and this.curTeamIndex is set to 0. I'm gonna look into that

Edit: Resolved in following commit. nevermind, it's still not quite right. I'll keep working on that issue

Edit^2: Okay I think this is better. All of my previously failing tests are now working. It's a little hacky, but there was some weird interactions with how teamIndex and this.curTeamIndex were getting set and it was easier to just identify this new case and bring it back in line with the existing code flow. I verified that default formats without teams are now showing up blank (instead of first team in builder) and default formats with teams are selecting the first team in that format. I also verified switching formats after initial load works as expected.

This should now be ready for review. Thanks!

@Slayer95
Copy link
Collaborator

This is missing logic in case Storage loads after the mainmenu.

  • See the Storage.whenPrefsLoaded() handler and augment it.
  • Format selection and LFAB should probably be disabled until prefs are loaded (or a timeout is exceeded.)

@bingingem
Copy link
Contributor Author

Thanks for the review and suggestions, Slayer. I'll take a look and update once I get those cases handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants